polkadot-sdk-docs: Use command_macro! #6624
Conversation
|
Can you please use |
@iulianbarbu, I used a slightly modified version of the macro used in |
| @@ -88,9 +84,8 @@ fn generate_chain_spec() { | |||
| .arg("preset_2") | |||
| .output() | |||
| .expect("Failed to execute command"); | |||
|
|
|||
| println!("Expected: {:?}", output.stdout); | |||
There was a problem hiding this comment.
reminder to remove once debugging is done
There was a problem hiding this comment.
Responding to this comment: #6624 (comment). Is there anything present at WASM_FILE_PATH on your machine? Also, would be great to check the exit code, because it surely failed if the output is empty.
Did you try running the same cmd in terminal to see how it fails or if it works?
There was a problem hiding this comment.
@iulianbarbu , you were right: nothing in the WASM_FILE_PATH environment variable, and yes the output is empty.
Running "../../../../../target/release/chain-spec-builder" list-presets -r $WASM_FILE_PATH from the command line after running the cargo build and setting the environment variable manually, I get the following error:
wasm blob shall be readable No such file or directory (os error 2)
There was a problem hiding this comment.
@iulianbarbu , the chain-spec-guide-runtime/ folder is missing from the target/release/wbuild/ folder.
There was a problem hiding this comment.
Can you do a cargo build --release -p chain-spec-guide-runtime and try again? Seems like tests depends on the runtime to be built prior to running it. Or even better, running a cargo build --release -p polkadot-sdk-docs should build everything that's needed most probably.
There was a problem hiding this comment.
@iulianbarbu , it worked!!!
Now the real work can start!!! 😃
There was a problem hiding this comment.
Additional question: shall I remove Command::new(...) wherever it is used, or only where #[docify::export] is used?
There was a problem hiding this comment.
I think that modifying wherever it is used should be fine (not a must, up to you), but mainly important to update in the portions with #[docify::export]. BTW, would be great to extract the cmd_lib based commands in separate functions where we have #[docify::export_content], leaving the assertion logic to a test which isn't exported in the docs. This example might help: https://github.com/paritytech/polkadot-sdk/blob/master/substrate/bin/utils/chain-spec-builder/tests/test.rs#L237.
docs/sdk/src/reference_docs/chain_spec_runtime/tests/chain_spec_builder_tests.rs
Outdated
Show resolved
Hide resolved
|
Bot fmt and label needed 👀 |
iulianbarbu
added
T11-documentation
| bash!( | ||
| chain-spec-builder { | ||
| "presets":[ | ||
| "preset_1", | ||
| "preset_2", | ||
| "preset_3", | ||
| "preset_4", | ||
| "preset_invalid" | ||
| ] | ||
| }, list-presets -r $path | ||
| ); |
There was a problem hiding this comment.
I have two remarks that apply to all the tests:
- Can we not assert inside the
bash!macro? Just want to avoid passing the expected value to it, and hence display it in the docs too. - Can we use
#[docify::export_content]instead of#[docify::export]? In this way we'll export only the function content, ignoring its signature.
The idea is to assert in the test which calls this function (e.g. list_presets()), and the function to return a String that can be parsed as JSON accordingly. In the end the command we'll appear in the docs like:
bash!(
chain-spec-builder list-presets -r $RUNTIME_PATH
)LMK if this doesn't make sense.
Description
Understood assignment:
Initial assignment description is in #6194 : find every occurrence of
#[docify::export]whereprocess:Commandis used → replace the use ofprocess:Commandbycommand_macro!Issues observed:
#[docify::export]&process:Commandare failing to begin with (tested & worked on:~/polkadot-sdk/docs/sdk/src/reference_docs/chain_spec_runtime/tests/chain_spec_builder_tests.rs)command_macrothe following file needed some modifications:~/.cargo/registry/src/index.crates.io-6f17d22bba15001f/command-macros-plugin-0.2.7/src/lib.rsI am running
cargo +nightly testfrom~/polkadot-sdk/docs/sdk/src/reference_docs/chain_spec_runtime/Same test failures are observed after changing from
process:Commandtocommand_macro!Some assistance is needed.